home *** CD-ROM | disk | FTP | other *** search
/ NetNews Offline 2 / NetNews Offline Volume 2.iso / news / comp / lang / c++-part2 / 17993 < prev    next >
Encoding:
Text File  |  1996-08-05  |  3.0 KB  |  111 lines
  1. Path: grimsel.zurich.ibm.com!usenet
  2. From: Keith Whittingham <wgk@zurich.ibm.com>
  3. Newsgroups: comp.lang.c++
  4. Subject: Woops!
  5. Date: Thu, 18 Apr 1996 14:57:05 -0700
  6. Organization: IBM Zurich Research Laboratory
  7. Message-ID: <3176BAB1.F52@zurich.ibm.com>
  8. NNTP-Posting-Host: pine.zurich.ibm.com
  9. Mime-Version: 1.0
  10. Content-Type: text/plain; charset=us-ascii
  11. Content-Transfer-Encoding: 7bit
  12. X-Mailer: Mozilla 2.01 (Win16; I)
  13.  
  14. I've just come across a strange bug which was causing a stack
  15. overflow which might be of interest to you next time you're 
  16. putting together a class. I have my own collection classes - 
  17. "What", you say, "Re-inventing the wheel?" Well yes but, it was
  18. necessary in order to have built in object persistance, and
  19. dynamic type checking, and the documentation on every other
  20. collection class library I've come across is lousy, oh, and of 
  21. course, it was fun. 
  22.  
  23. Anyway the base of the collection is a linked list. When a 
  24. collection, such as a sorted list, is destructed, if destroys 
  25. it's members. This behaviour is inherited from a ABCs Collection
  26. and Node. The relevant bits look like this...
  27.  
  28.  
  29. class Node
  30. {
  31.   public:
  32.     Node() { Nic = 0; }
  33.     virtual ~Node() { if(Nic) delete Nic; }
  34.   private:
  35.     Node *Nic;  // Next in chain
  36. };
  37.  
  38. class Collection:
  39.   public Node
  40. {
  41.   public:
  42.     Collection() { Fic = 0; }
  43.     virtual ~Collection() { if(Fic) delete Fic; }
  44.     virtual void Add(Node &n) { /* not relevant code */ }
  45.   private:
  46.     Node  *Fic; // First in chain
  47. };
  48.  
  49.  
  50. All pretty innocuous stuff really - well... A wolf in lambs
  51. clothing.
  52.  
  53.   Collection *c = new Collection;
  54.   for(int i = 0; i < 2000; i++)
  55.   {
  56.     c.Add(*new Node);
  57.   }
  58.   delete c; // Bang! (well sometimes)
  59.  
  60. Why? Well the list of nodes is freed by deleteing the first
  61. in the chain, that in turn deletes the next and so on. The problem
  62. is that the call is, effectively recursive and for each node 
  63. in the chain, another return address and another address of 'this'
  64. is pushed onto the statck. This carries on until the stack
  65. runs out - as does your program.
  66.  
  67. The solution - simple have the collection delete the members
  68. rather than have the chain delete itself...
  69.  
  70. class Node
  71. {
  72.   public:
  73.     Node() { Nic = 0; }
  74.     virtual ~Node() { }        ///
  75.   private:
  76.     Node *Nic;  // Next in chain
  77. };
  78.  
  79. class Collection
  80. {
  81.   public:
  82.     Collection() { Fic = 0; }
  83.     ~Collection()
  84.     {
  85.        Node *n, *Tmp;          ///
  86.        n = Fic;                ///
  87.        while(n)                ///
  88.        {                       ///
  89.           Tmp = n->Nic;        ///
  90.           delete n;            ///
  91.           n = Tmp;             ///
  92.        }                       ///
  93.     }                          ///
  94.     virtual void Add(Node &n) { /* not relevant code */ }
  95.   private:
  96.     Node  *Fic; // First in chain
  97. };
  98.  
  99.  
  100. Had the statement "delete Nic;" been written Nic::~Node() it
  101. probably would have been clear from the outset that the 
  102. call was recursive and since it's part of a collection class
  103. the depth of recursion can not be ascertained. In retrospect
  104. it's not really a bug, just inappropriate code.
  105.  
  106. ...OK it's a bug.
  107.  
  108. -- 
  109. Keith Whittingham
  110. wgk@zurich.ibm.com
  111.